fix(googlechat): supported keyless ADC and reliable send-once by chaodu-obk[bot] · Pull Request #1513 · openabdev/openab

chaodu-obk · 2026-08-31T20:01:09Z

Summary

This PR carries #1512 plus focused maintainer fixes for Google Chat authentication and delivery reliability.

It adds supported keyless authentication with two distinct service accounts: the attached runtime service account obtains a metadata credential and impersonates a dedicated Google Chat service account for the chat.bot scope. Runtime/target equality is rejected before the base token is requested because Google prohibits access-token self-impersonation.

Google Chat remains send-once, but delivery acknowledgement is independent from cosmetic streaming. Configured standalone and unified paths report API, authentication, channel, and timeout failures to core instead of returning synthetic success.

Architecture

Keyless ADC

Read the attached runtime service-account email from the GCE metadata server.
Validate that the configured target is a distinct user-managed service-account email.
Obtain the runtime service account's short-lived metadata token.
Call IAM Credentials generateAccessToken for the distinct target with chat.bot.
Cache the target token and use it for Google Chat API calls.

The runtime identity needs roles/iam.serviceAccountTokenCreator on the target service account. Existing service-account-key and static-token paths remain available. Auth precedence is: a successfully loaded service-account key, then ADC, then an explicitly configured static token.

Send-once delivery acknowledgement

Core retains a request_id for Google Chat normal replies even though cosmetic streaming is disabled.
The adapter uses a 30-second absolute delivery deadline inside core's 35-second acknowledgement window.
Queue wait, token resolution, and every sequential chunk are included in the adapter deadline.
The unified path receives the delivery Result directly; the standalone path receives a correlated GatewayResponse.
Partial multi-chunk failures report the failed chunk and number of chunks already delivered.

Review Contract

This section is the exclusive merge-review contract for this PR. Review findings must map to one of the blocker rules below. A useful observation that does not map to a blocker rule is a non-blocking follow-up and must not extend the finish line.

Goal

Deliver a supported two-identity keyless Google Chat auth path and reliable send-once failure reporting without removing existing auth methods or changing unrelated platforms.

In scope

Google Chat service-account-key, ADC, and static-token selection boundaries.
Distinct runtime/target identity enforcement and IAM Credentials transport safety.
Send-once normal-reply acknowledgement on standalone and unified paths.
Bounded token and message-delivery failure behavior.
Google Chat configuration, Helm Workload Identity attachment, operator docs, and focused regression coverage.
Backward compatibility for existing Google Chat and Helm deployments.

Non-goals

A generic cross-platform auth-mode framework.
Consolidating GoogleChatTokenCache and MetadataTokenSource; tracked by Consolidate googlechat token-source cache machinery (review F29 follow-up) #1514.
Keyless auth for platforms other than Google Chat.
A negotiated cross-platform capability protocol.
General Google Chat throughput optimization beyond the accepted residual risks below.
Removing service-account-key or static-token authentication.

Exclusive blocker rules

A finding blocks this PR only when at least one rule is satisfied:

Acceptance criterion: one of the unchecked criteria below is not met.
Changed-code correctness or security: the PR introduces a reproducible correctness, authentication, authorization, data-loss, or credential-exposure defect on a supported path.
Required gate: a required exact-head CI check fails or the reviewed SHA cannot be verified.
Backward compatibility: the PR silently changes an existing supported deployment's identity, permissions, or configuration semantics without an explicit opt-in or documented migration.

Every blocking finding must cite its rule, affected supported path, concrete evidence, and a testable requested change. Architecture preference alone is not a blocker.

Non-blocking follow-ups

The following do not block when the implementation is correct under the criteria above:

Readability, naming, comment, formatting, DRY, or documentation-completeness nits.
Defense-in-depth suggestions with no reachable untrusted input path.
Performance or scalability improvements beyond the accepted residuals below.
Baseline issues that this PR does not worsen.
Alternative abstractions or refactors already covered by a follow-up.
Adding an additional CI job when all required exact-head checks pass and the relevant tests have been explicitly validated for this review.

Non-blocking observations should be recorded once, deduplicated, and moved to a follow-up rather than causing another review round.

Accepted Residual Risks

The owner accepts these for this PR; they are follow-ups unless evidence shows a blocker-rule violation:

Google Chat sends are conservatively serialized by one adapter-wide delivery lock. Per-space FIFO workers and throughput isolation are follow-up optimizations.
Multi-chunk delivery can be partial. Errors identify the failed chunk and delivered count; dedicated Retry-After and per-space pacing are follow-ups.
SA-key failed-refresh retries are individually bounded by the 10-second token request timeout. Shared cache/cooldown consolidation remains part of Consolidate googlechat token-source cache machinery (review F29 follow-up) #1514.
Ten local mock-server/filesystem integration tests are marked ignored and were explicitly run during validation. Making them a separate required CI job is a follow-up.
IAM error classification is best-effort; unrecognized bodies remain unclassified with a truncated raw body.
Token request (10 seconds), adapter delivery (30 seconds), core acknowledgement (35 seconds), and ADC failed-refresh cooldown (30 seconds) are fixed constants.
A configured static fallback is opaque and may represent a different identity; degradation logs an explicit possible identity switch.
Missing/invalid ADC target configuration logs an error and disables ADC; an explicitly configured usable fallback may continue.
Google Chat delete remains an intentional no-op.

Acceptance Criteria

Completed:

ADC requires a configured target service-account email and propagates it through config, environment, and Helm.
Runtime and target service accounts must differ; equality fails before metadata base-token and IAM mint calls.
Metadata requests are no-proxy/no-redirect; IAM requests remain proxy-aware and no-redirect.
Blank or whitespace-only tokens are rejected at static, SA-key exchange, metadata, and IAM boundaries.
Configured Google Chat normal replies preserve delivery acknowledgement while remaining send-once.
Configured standalone and unified delivery failures propagate to core.
Queue wait, token resolution, and all chunks fit within the adapter's absolute deadline.
Exact head 0d142de33204d98169d7171926a35b9b3d68c38d passes all required checks: 39 success and one intentional skip.
The owner-authorized NIT batch was independently re-reviewed with no new regression.

Resolved frozen blockers:

Key validation (rules 1 and 2): invalid RSA PEM fails during SA-key construction, so it cannot suppress a configured ADC fallback until first delivery.
Helm upgrade identity (rules 1 and 4): an empty gateway ServiceAccount value preserves the Kubernetes default identity and does not inherit agent/global values.
Missing standalone adapter acknowledgement (rules 1 and 2): a reply carrying request_id receives an immediate structured failure when the Google Chat adapter is unavailable.

Follow-ups

These are explicitly non-blocking for this PR:

Consolidate googlechat token-source cache machinery (review F29 follow-up) #1514: consolidate the two Google Chat token caches and decide the long-term auth-mode abstraction.
Replace adapter-wide serialization with per-space FIFO scheduling and dedicated pacing/backoff if production load requires it.
Promote the ignored local integration suite into a dedicated required CI job.
Replace platform-name capability lists with a negotiated cross-platform capability protocol.

Scope freeze and review closure

The three frozen blocker criteria are resolved in 3771053f; 0d142de3 fixes the only CI regression from that delta.
Final review examines only those fixes, their regression surface, and compliance with the frozen Acceptance Criteria.
Reports against an older SHA are discarded, not merged into the current decision.
A new blocker requires evidence that the latest fix delta itself violates one of the exclusive blocker rules.
Once the exact-head round is terminal, late observations on the unchanged SHA are supplemental and do not create a new round or alter status unless the owner explicitly promotes them.
Nits are batched at most once; the NIT batch is complete in f59cbf9c.

Current review state

The prior broad review reported six important items. Under this bounded contract:

Frozen blockers: all three resolved in 3771053f, with the Clippy regression fixed in 0d142de3.
Accepted follow-ups: per-space delivery scheduling/rate limiting, a required CI job for ignored integration tests, and SA-key refresh-cooldown consolidation.

The final exact-head focused review is ready at 0d142de33204d98169d7171926a35b9b3d68c38d.

Commits

bd62ee49 - resolves timeout, token-boundary, precedence, documentation, readability, and operability findings.
904f626c - makes delete an explicit no-op.
fa9d58a9 - locks token and edit-resource boundaries.
3a1ce860 - implements distinct-target ADC, delivery acknowledgement, fallback diagnostics, and ADC refresh cooldown.
3b08312 - hardens metadata transport with no-proxy and fail-loud construction.
61153772 - separates metadata/IAM proxy policy, validates static-token boundaries, and improves ADC diagnostics.
31cea79d - propagates unified delivery outcomes, bounds delivery, validates target emails, and wires gateway ServiceAccounts.
b298e0a8 - includes queue wait in the deadline and preserves partial-timeout context.
f59cbf9c - resolves the owner-approved review NIT batch.
3771053f - resolves the three frozen contract blockers with focused tests and docs.
0d142de3 - explicitly detaches the Google Chat delivery task to satisfy Clippy.

Validation

At exact head 0d142de33204d98169d7171926a35b9b3d68c38d:

Review Contract validation passed.
Workspace check/clippy/tests and feature-specific checks passed.
Helm unit tests and platform-schema conformance passed.
Standard and unified Docker smoke matrices passed.
All 40 reported check runs were terminal: 39 success and one intentional operator skip.
The frozen-blocker delta passed git diff --check; OpenSSL validated the 2048-bit RSA fixture; focused correctness/test audits returned LGTM; exact-head CI compiled and exercised the Rust and Helm changes.

This validation records the current head only. Any subsequent code change creates a new exact head and must rerun the required checks.

Keyless ADC (MetadataTokenSource): mint a chat.bot-scoped token from the workload's own GCP identity — GCE metadata (SA email + base token) -> IAM Credentials generateAccessToken (self-impersonation). No SA key file. Config [googlechat].use_adc / GOOGLE_CHAT_USE_ADC; auth precedence SA key > ADC > static token; cache under the IAM-granted expireTime (fallback 3600s). Send-once for Google Chat: its write rate limit is 1/sec/space (create+patch+delete combined) so per-token streaming edits 429, and the unified adapter returns a synthetic message id that patch can't target (404). googlechat added to NON_STREAMING_PLATFORMS (renamed from NON_EDITABLE_PLATFORMS); resolve_streaming forces send-once on both the embedded dispatch (stream_prompt_blocks) and WebSocket gateway paths. Also: Dockerfile.claude OPENAB_BUILD_FEATURES arg, Helm googleChat.useAdc value, docs + config-first conformance entry + googlechat.toml schema record. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- F15: bound every token-mint request (SA-key exchange, metadata, IAM Credentials) with a 10s TOKEN_REQUEST_TIMEOUT so a hung connection cannot stall senders behind the cache write lock or defeat the ADC -> static token degradation path - F16: reject empty/whitespace minted tokens at all three extraction sites (SA-key exchange, metadata base token, generateAccessToken) so a malformed response follows the degradation path instead of being cached as valid - F17: correct the shorthand precedence wording in config.toml.example, config.rs, config-reference.md, google-chat.md env table, and values.yaml to name the configured-but-unloadable-key -> ADC fallback - F19: refuse edit_message for non-resource-name (synthetic unified_) ids locally instead of sending a doomed patch (400 INVALID_ARGUMENT) - F20: cross-reference the two sibling streaming gates (resolve_streaming / platform_supports_streaming) in both docs - F21: document get_token precedence and its asymmetric failure behavior at the function - F22: replace from_parts' five positional args with a named GoogleChatParts struct; all call sites and tests name their fields - F23: install metadata_source only when no SA key loaded, so the code encodes the precedence it documents - F24: drop private review-numbering labels (F1/F2/F4/F5) from source comments and test comments - F25: fix the self-contradictory 'immutable after creation' GCE scope wording in docs/google-chat.md Option C - F26: identify the orphaned Secret (agentFullname convention + discovery commands) in the key-to-ADC migration note - F27: log the resolved service-account identity on successful mint - F28: classify generateAccessToken failures (insufficient_scope / missing_role / api_not_enabled) in the error string New regression tests: loaded-key-suppresses-ADC-source, blank-minted- token rejection (wiremock), synthetic-id edit_message no-op (wiremock, expect(0)), and error-classification table.

Route delete_message with the other unsupported Google Chat commands so it returns before token resolution, logging, or network work instead of falling through to the empty-send response path. Add a regression test that distinguishes the old fallthrough behavior and update the platform schema feature/quirk notes to document the explicit no-op.

Share one non-whitespace token validator across the SA-key, metadata, and IAM response paths and cover empty/whitespace/valid values in a table test. Require exact spaces/{space}/messages/{message} edit targets, cover malformed resource shapes, and assert the valid edit path issues exactly one PATCH.

chaodu-obk

Important

CHANGES REQUESTED ⚠️ - The keyless ADC flow violates Google's self-impersonation contract, and send-once mode suppresses Google Chat delivery failures.

Consolidated review: #1513 (comment)

GitHub event: COMMENT - self-review delivery only; this is not an approval.

- require a distinct adc_target_service_account for keyless ADC; reject runtime/target equality before requesting the metadata base token because Google prohibits access-token self-impersonation - plumb GOOGLE_CHAT_ADC_TARGET_SERVICE_ACCOUNT through config, env, Helm, docs, schema, and config-first conformance; log both runtime and target SAs - classify documented FAILED_PRECONDITION self-impersonation errors - decouple normal-reply acknowledgements from cosmetic streaming so Google Chat remains send-once but carries/awaits request_id; promised ack failures, channel closure, and timeout now fail closed instead of reporting gw_sent - remove unverifiable ADC/static-token identity-equivalence claims and log static fallback as a possible identity switch - add 30s failed-refresh cooldown so queued senders reuse a still-valid token instead of serially repeating metadata/IAM timeouts - add regression tests for distinct-target enforcement, ack error propagation, and refresh retry suppression

The MetadataTokenSource client carries the plaintext GCE metadata bearer. Two hardening fixes on its construction: - Add .no_proxy(): reqwest honors HTTP(S)_PROXY by default, which could route the metadata access-token response through an operator/attacker proxy hop. Disable proxies so the bearer never leaves the metadata path. - Replace .build().unwrap_or_default() with .expect(): on a builder error unwrap_or_default() yields a DEFAULT client that follows redirects and honors proxies, silently defeating the no-redirect/no-proxy guarantee this source exists to uphold. Fail loud at construction instead. Addresses supplementary review findings on #1513 (F3/F4).

Brings in the Hermes removal (#1523) so this branch no longer builds or CI-tests the Hermes image — the flaky raw.githubusercontent.com 429 smoke failures were unrelated to this PR. No conflicts (ADC changes and the Hermes deletions touch disjoint regions of values.yaml / config-reference.md).

Close the remaining review gaps in the corrected keyless Google Chat flow: - share a direct Result-returning delivery path between standalone WebSocket handling and the unified in-process adapter, so unified auth/API failures reach core instead of becoming synthetic success; preserve real message IDs - bound the complete delivery to 30s (inside core's 35s ack window), add 10s per-mutation timeouts, and stop on the first failed chunk with explicit partial-delivery context - validate/normalize user-managed target SA emails before IAM URL construction, rejecting numeric unique IDs and path/query/trailing-dot aliases; keep the MetadataTokenSource constructor private behind that invariant - attach an existing ServiceAccount to gateway pods with precedence gateway > per-agent > global, document GKE Workload Identity setup, and add Helm tests (including numeric-looking KSA names rendered as strings) - classify the proxy-routing test as ignored integration, accept common GOOGLE_CHAT_USE_ADC case/whitespace forms, and avoid false failures when an accepted 2xx response omits a message resource name Validated on macmini: workspace all-feature check, gateway+unified clippy with -D warnings, gateway tests (332 pass/1 ignored), ignored proxy test explicitly, unified/core regressions, feature-off core check, and Helm unittest 46/46.

chaodu-obk

Important

CHANGES REQUESTED ⚠️ - Invalid key material can suppress ADC, and Helm inheritance can silently change the gateway identity on upgrade.

Consolidated review: #1513 (comment)

GitHub event: COMMENT - self-review delivery only; this is not an approval.

Close the final review gaps in standalone Google Chat delivery: - spawn Google Chat handling immediately from the WebSocket receive loop so replies no longer wait behind an inline send while core's ack clock runs - serialize actual sends with an adapter-owned delivery mutex, starting the absolute 30s deadline before lock acquisition so queue + token + chunks are all inside the 35s core acknowledgement window - replace the context-free outer timeout with phase/chunk-aware deadline waits; queue, token, and partial-chunk timeout errors now retain delivery context - add queue-wait and delayed partial-timeout regressions - classify every PR-added loopback-network/filesystem test as ignored integration and run all ten explicitly in validation Validated on macmini: workspace all-feature check, gateway/unified clippy with -D warnings, 324 normal gateway tests + 10 ignored integrations, and unified failure propagation regression all pass.

sebastian-hsu · 2026-09-04T22:32:19Z

Correcting the record on the self-impersonation question, since I argued the other side on #1512 and that argument was wrong.

I was wrong, and specifically about the reasoning — not only the conclusion.

My earlier position was that the flow was acceptable because the runtime SA holds roles/iam.serviceAccountTokenCreator on itself and the requested scope differs (cloud-platform base → chat.bot), which I described as a supported different-scope pattern. I did not read the cited documentation before making that claim. Having now read it, the scope-based justification has no basis:

"Using a short-lived credential for a service account to generate a new access token for the service account" is prohibited; the sole exception is a self-signed JWT.
FAILED_PRECONDITION: You can't create a token for the same service account that you used to authenticate the request.

The criterion is the caller's credential type plus caller-identity == target-identity. Scope is not part of it. My argument reconciled "the docs say no" with "it works in practice" by inventing a mechanism, and I stated that invention as fact. That was the error.

On the empirical observation. It remains true that the flow minted chat.bot tokens successfully, repeatedly, over several days. The correct reading of that is "not currently enforced in this configuration", not "supported" — and for a production authentication path, undocumented behaviour that works today is not something to build on. Both @canyugs's live result and mine were real; neither outranks the vendor contract.

What we did about it. We adopted this PR's design in production rather than continuing to argue for ours:

Both of our Google Chat agents now run 3a1ce860 with adc_target_service_account set to a distinct Chat SA.
Verified end to end: each agent mints a chat.bot token via generateAccessToken against its distinct target and delivers Chat replies. Logs show runtime_service_account and target_service_account as different identities, as intended.
A cross-project target works. One agent's runtime SA lives in one project and its Chat SA in another, with serviceAccountTokenCreator granted on the target across projects. This is a configuration a diff review cannot cover, so flagging it as a datapoint.
No Chat-app console change was required when switching an app's sending identity to the new SA — granting the impersonation binding was sufficient.
The acknowledgement rework in this commit has also been running in production since; we took it as-is rather than deferring it, so F2 is exercised too.

A design point that settles this independently of the contract. Self-impersonation cannot express a cross-project target at all: the adapter can only mint for the identity the metadata server returns. Any deployment whose Chat app SA lives in a different project requires this PR's distinct-target model. We hit exactly that constraint, which is what pushed us onto this design before I had re-read the documentation.

I'm not commenting on the currently open blockers — those are the maintainers' call. Happy to supply further detail from the production deployment if it is useful for verification.

chaodu-obk · 2026-09-04T22:35:30Z

Note

LGTM ✅ - All frozen Review Contract criteria are resolved at exact head 0d142de33204d98169d7171926a35b9b3d68c38d, and every required exact-head check is green.

What This PR Does

This PR adds supported keyless Google Chat authentication through a runtime service account impersonating a distinct Chat-app service account. It keeps Google Chat send-once while preserving delivery failure reporting across standalone and unified paths, and hardens credential boundaries, timeouts, Helm configuration, diagnostics, documentation, and regression coverage.

How It Works

The gateway validates a distinct target service-account email, reads the attached runtime identity and metadata credential, and calls IAM Credentials generateAccessToken for chat.bot. Configured normal sends use one bounded delivery implementation. The unified path receives its Result directly; the standalone path receives a correlated GatewayResponse within core's acknowledgement window.

Findings

#	Severity	Outcome	Location
F1	🟢 Resolved	SA-key construction now validates RSA PEM immediately; invalid PEM cannot suppress the configured ADC fallback.	`crates/openab-gateway/src/adapters/googlechat.rs:1056`
F2	🟢 Resolved	Empty `gateway.serviceAccountName` now preserves the Kubernetes default identity and does not inherit agent/global values.	`charts/openab/templates/gateway.yaml:7`
F3	🟢 Resolved	A standalone reply with no Google Chat adapter now emits an immediate correlated failure instead of waiting 35 seconds.	`crates/openab-gateway/src/lib.rs:958`
F4	🟢 Praise	The supported two-identity ADC design, proxy split, token boundaries, exact deadlines, unified propagation, and bounded review contract are strong.	-

Resolution Details

F1: Construction-time PEM validation

GoogleChatTokenCache::new now parses the RSA key before the cache is considered loaded. Tests cover invalid PEM rejection, valid PEM acceptance, ADC installation after invalid PEM, and valid-key precedence. The checked-in 2048-bit fixture was independently validated with OpenSSL.

F2: Helm upgrade identity compatibility

The gateway template now reads only the explicit gateway ServiceAccount value. Helm tests prove that chart-global and per-agent values do not affect the gateway, while explicit gateway configuration, Workload Identity pairing, and numeric-looking names still render correctly. README, values, config reference, and Google Chat docs match the behavior.

F3: Missing-adapter failure acknowledgement

The production WebSocket dispatch seam now routes missing-adapter failures through the same structured response helper used by configured delivery. The response preserves request_id, sets success=false, and returns a clear configuration error. Tests cover both the payload helper and the production dispatch seam.

Addressing External Reviewer Feedback

@sebastian-hsu

The prior same-service-account justification was incorrect; the distinct-target design follows the vendor contract and has been verified in production, including cross-project impersonation.

Accepted and corroborated: this evidence supports the selected two-identity architecture. The current exact head retains the distinct-target guard before base-token retrieval. The reported production success for cross-project impersonation and configured delivery acknowledgement is useful operational evidence and introduces no new blocker.

All six existing inline review threads are now resolved. Accepted residual risks and Follow-ups remain explicitly non-blocking under the frozen PR contract.

Baseline Check

PR opened: 2026-08-31
Declared base and merge base: main at 7cbe5c9327c697d0c7be89dd8b028de724a3afb0
Reviewed head: 0d142de33204d98169d7171926a35b9b3d68c38d
Diff: 17 files, 2,178 additions, 279 deletions
Main already has: the Google Chat adapter plus service-account-key and static-token auth.
Net-new value: supported distinct-target ADC, reliable send-once acknowledgement, bounded delivery behavior, explicit gateway Workload Identity attachment, stronger diagnostics, docs, and tests.

Validation

Review Contract validation passed with every frozen criterion checked.
Exact-head CI: 40 terminal runs, 39 success, one intentional operator skip.
Workspace check, Clippy with warnings denied, tests, and feature-specific checks passed.
Helm unit tests, platform-schema conformance, standard smoke tests, builder, and unified smoke matrix passed.
git diff --check passed for the frozen-blocker delta.
OpenSSL validated the 2048-bit RSA fixture.
Focused correctness and test-contract audits returned LGTM; a requested route-seam regression was added before commit.
The intermediate Clippy failure at 3771053f was fixed in 0d142de3; the final exact head is green.

Independent Review Coverage

Focus	Outcome
Frozen blocker correctness	LGTM - all three implementation paths satisfy the contract
Focused tests and docs	LGTM - direct regression coverage and consistent operator docs
Final exact-head CI	Green - all required non-skipped checks succeeded
External production evidence	Supports the distinct-target and acknowledgement design

What's Good (🟢)

The auth flow follows Google's documented distinct-principal impersonation contract.
Credential-bearing metadata and IAM calls use separate bounded transport policies.
Invalid keys and identity configuration fail at clear boundaries with actionable diagnostics.
Standalone and unified configured-delivery paths share consistent failure semantics.
The PR now has a frozen stopping rule: accepted residuals remain Follow-ups rather than reopening review scope.

5. Three Reasons We Might Not Need This PR

Existing key-based authentication works - deployments without a key-elimination requirement can avoid metadata and IAM Credentials complexity.
Auth and delivery are separable concerns - they could have been reviewed and rolled back independently.
A shared auth refactor is tracked - Consolidate googlechat token-source cache machinery (review F29 follow-up) #1514 will eventually consolidate the parallel token-cache implementations.

These trade-offs are accepted and do not violate the frozen Review Contract.

sebastian-hsu and others added 2 commits August 31, 2026 08:44

chaodu-obk Bot requested a review from thepagent as a code owner August 31, 2026 20:01

chaodu-obk Bot mentioned this pull request Aug 31, 2026

feat(googlechat): keyless ADC auth + send-once for the unified adapter #1512

Closed

11 tasks

chaodu-obk Bot added 2 commits August 31, 2026 20:40

chaodu-obk Bot changed the title ~~fix(googlechat): address PR #1512 review findings F15-F28~~ fix(googlechat): resolve PR #1512 review findings Aug 31, 2026